Skip to content

refactor: update SettingsPage to handle async params #257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
izadoesdev merged 2 commits into from
Jan 4, 2026
Merged

refactor: update SettingsPage to handle async params #257

izadoesdev merged 2 commits into from
Jan 4, 2026

Conversation

@sbansal1999
Copy link
Contributor

@sbansal1999 sbansal1999 commented Jan 4, 2026 •
edited by coderabbitai bot
Loading

Description

Since the params are sent as a promise, they have to be awaited before they are read on a page. This PR awaits that promise and also updates the navigation category function to use includes rather than startsWith to detect the category.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • Refactor

    • Updated settings page component initialization for better performance handling.

  • Bug Fixes

    • Improved navigation category detection accuracy for better active page highlighting.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Jan 4, 2026

@sbansal1999 is attempting to deploy a commit to the Databuddy OSS Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2026 •
edited
Loading

Walkthrough

Two files updated: a page component transitions to async and updates its params prop type to Promise<{ id: string }>, requiring await before accessing the id; a navigation utility changes pathname matching from prefix-based (startsWith) to substring-based (includes).

Changes

Cohort / File(s) Summary
Settings Page Async Params
apps/dashboard/app/(main)/websites/[id]/settings/page.tsx		 SettingsPage converted to async function; params prop type changed from { id: string } to Promise<{ id: string }>, requiring await to resolve website ID before redirect logic.
Navigation Matching Logic
apps/dashboard/components/layout/navigation/navigation-config.tsx		 getDefaultCategory pathname matching switched from prefix-based (startsWith) to substring-based (includes), expanding pattern detection to any substring occurrence.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: updating SettingsPage to handle async params, which aligns with the primary modification in the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a021465 and bcc0dd7.

📒 Files selected for processing (2)
  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx,js,jsx,vue}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

**/*.{ts,tsx,js,jsx,vue}: Never block paste in <input> or <textarea> elements
Enter submits focused text input; in <textarea>, ⌘/Ctrl+Enter submits; Enter adds newline
Compatible with password managers and 2FA; allow pasting one-time codes

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{html,htm,tsx,jsx,vue}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

**/*.{html,htm,tsx,jsx,vue}: Use autocomplete attribute with meaningful name; use correct type and inputmode
Disable spellcheck for email/code/username inputs using spellcheck="false"
Links are <a> or <Link> components for navigation; support Cmd/Ctrl/middle-click
Use polite aria-live for toasts and inline validation feedback
Autofocus on desktop when there's a single primary input; rarely on mobile to avoid layout shift
<title> tag must match current context
Use redundant status cues (not color-only); icon-only elements have text labels
Use accurate accessible names via aria-label; mark decorative elements with aria-hidden; verify in Accessibility Tree
Icon-only buttons must have descriptive aria-label
Prefer native semantics (button, a, label, table) before ARIA
Use non-breaking spaces to glue terms: 10&nbsp;MB, ⌘&nbsp;+&nbsp;K, Vercel&nbsp;SDK
Preload only above-the-fold images; lazy-load the rest

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

**/*.{ts,tsx,js,jsx}: Trim values to handle text expansion and trailing spaces in form submissions
URL reflects state (deep-link filters/tabs/pagination/expanded panels); prefer libraries like nuqs
Back/Forward buttons restore scroll position
Delay first tooltip in a group; subsequent peers have no delay
Use locale-aware formatting for dates, times, numbers, and currency
Batch layout reads/writes; avoid unnecessary reflows/repaints
Virtualize large lists using libraries like virtua

**/*.{ts,tsx,js,jsx}: Don't use accessKey attribute on any HTML element.
Don't set aria-hidden="true" on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like <marquee> or <blink>.
Only use the scope prop on <th> elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assign tabIndex to non-interactive HTML elements.
Don't use positive integers for tabIndex property.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include a title element for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
Assign tabIndex to non-interactive HTML elements with aria-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include a type attribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden).
Always include...

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{css,scss,sass,less,html,htm,tsx,jsx,vue}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

**/*.{css,scss,sass,less,html,htm,tsx,jsx,vue}: No dead zones on checkboxes/radios; label and control share one generous hit target
Use scroll-margin-top on headings for anchored links; include 'Skip to content' link; use hierarchical <h1>–<h6>
Prevent Cumulative Layout Shift (CLS) from images by using explicit dimensions or reserved space

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{ts,tsx,js,jsx,css,scss,sass,less}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

**/*.{ts,tsx,js,jsx,css,scss,sass,less}: During drag operations, disable text selection and set inert on dragged element and containers
Animations must be interruptible and input-driven; avoid autoplay

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{tsx,jsx,vue}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

Don't ship the schema—visuals may omit labels but accessible names must still exist

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

Prefer uncontrolled inputs; make controlled loops cheap (optimize keystroke cost)

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{ts,tsx,jsx}: Use semantic elements instead of role attributes in JSX.
Don't use unnecessary fragments.
Don't pass children as props.
Don't use the return value of React.render.
Make sure all dependencies are correctly specified in React hooks.
Make sure all React hooks are called from the top level of component functions.
Don't forget key props in iterators and collection literals.
Don't destructure props inside JSX components in Solid projects.
Don't define React components inside other components.
Don't use event handlers on non-interactive elements.
Don't assign to React component props.
Don't use both children and dangerouslySetInnerHTML props on the same element.
Don't use dangerous JSX props.
Don't use Array index in keys.
Don't insert comments as text nodes.
Don't assign JSX properties multiple times.
Don't add extra closing tags for components without children.
Use <>...</> instead of <Fragment>...</Fragment>.
Watch out for possible "wrong" semicolons inside JSX elements.
Make sure void (self-closing) elements don't have children.
Don't use target="_blank" without rel="noopener".
Don't use <img> elements in Next.js projects.
Don't use <head> elements in Next.js projects.

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't return a value from a function with the return type 'void'.
Don't use the TypeScript directive @ts-ignore.
Don't use TypeScript enums.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the ! postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use either T[] or Array<T> consistently.
Initialize each enum member value explicitly.
Use export type for types.
Use import type for types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.

**/*.{ts,tsx}: Do NOT use types 'any', 'unknown' or 'never'. Use proper explicit types
Suffix functions with 'Action' in types, like 'type Test = { testAction }'

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
!(**/pages/_document.{ts,tsx,jsx})**/*.{ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{ts,tsx,html,css}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

Use bun build <file.html|file.ts|file.css> instead of webpack or esbuild for bundling

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/01-MUST-DO.mdc)

**/*.{jsx,tsx}: When using 'text-right' utility class, always add 'text-balance' to prevent poor text layout
Think about mobile responsiveness when doing any UI tasks or changes
Always use 'rounded' Tailwind class, never 'rounded-xl' or 'rounded-md'
Don't use lucide for icons, ONLY use phosphor icons. Use width='duotone' for most icons, use fill for arrows, and don't add width attribute for plus icons
Decouple state management, data transformations, and API interactions from the React lifecycle in View Components
Simplify data flow to eliminate prop drilling and callback hell in components
Prioritize modularity and testability in all components
ALWAYS use error boundaries properly in React applications
Use 'Icon' suffix at the end of phosphor react icon imports, like CaretIcon not Caret. This is the default import, NOT as a named import
Almost NEVER use useEffect unless it's critical

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/01-MUST-DO.mdc)

**/*.{js,jsx,ts,tsx}: Split off components, utils, and reusable code to ensure better loading speed and less complexity
Use lower-case-like-this naming convention for variables, functions, and identifiers
NEVER add placeholders, mock data, or anything similar to production code
Use Dayjs for date handling, NEVER use date-fns. Use Tanstack query for hooks, NEVER use SWR
Use json.stringify() when adding debugging code
Never use barrel exports or create index files

Files:

  • apps/dashboard/app/(main)/websites/[id]/settings/page.tsx
  • apps/dashboard/components/layout/navigation/navigation-config.tsx
🧠 Learnings (2)
📓 Common learnings 
Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-12-02T22:57:07.791Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Make sure async functions actually use await.

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/guidelines.mdc:0-0
Timestamp: 2025-12-02T22:55:00.415Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : URL reflects state (deep-link filters/tabs/pagination/expanded panels); prefer libraries like `nuqs`

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-12-02T22:57:07.791Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Make sure Promise-like statements are handled appropriately.

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-12-02T22:57:07.791Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Make sure all anchors are valid and navigable.

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-12-02T22:57:07.791Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Don't use async functions as Promise executors.

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/guidelines.mdc:0-0
Timestamp: 2025-12-02T22:55:00.415Z
Learning: Applies to **/*.{html,htm,tsx,jsx,vue} : Links are `<a>` or `<Link>` components for navigation; support Cmd/Ctrl/middle-click

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-12-02T22:57:07.791Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Don't use await inside loops.

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/01-MUST-DO.mdc:0-0
Timestamp: 2025-12-25T14:32:46.935Z
Learning: Applies to **/*.{jsx,tsx} : Simplify data flow to eliminate prop drilling and callback hell in components

Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-12-02T22:57:07.791Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Don't use callbacks in asynchronous tests and hooks.
📚 Learning: 2025-12-02T22:55:00.415Z 
Learnt from: CR
Repo: databuddy-analytics/Databuddy PR: 0
File: .cursor/rules/guidelines.mdc:0-0
Timestamp: 2025-12-02T22:55:00.415Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : URL reflects state (deep-link filters/tabs/pagination/expanded panels); prefer libraries like `nuqs`

Applied to files:

  • apps/dashboard/components/layout/navigation/navigation-config.tsx
🔇 Additional comments (2)
apps/dashboard/app/(main)/websites/[id]/settings/page.tsx (1)

3-10: LGTM! Correctly implements Next.js 15 async params pattern.

The changes properly handle the async params per Next.js 15's breaking changes. The function awaits the params promise, destructures the id, and uses it for the redirect.

Based on learnings, the async function correctly uses await and handles the Promise appropriately.

apps/dashboard/components/layout/navigation/navigation-config.tsx (1)

414-421: The examples of false positives provided are not accurate for this codebase.

The patterns "/organizations", "/billing", and "/settings" all include a leading slash. Examples like /websites/my-billing-site/analytics would not match because they don't contain the substring "/billing" (only "billing" without the slash). Similarly, /websites/settings-panel/map wouldn't match because it lacks the "/settings" substring.

The actual pathnames passed to this function from usePathname() (which returns only the path portion without query strings) are structured routes like /organizations/members, /billing/history, and /settings/account, all of which correctly match their respective patterns with includes().

That said, using includes() instead of path-segment-aware matching (like checking the first segment after splitting by /) is semantically imprecise for route classification, even if it works correctly in the current codebase structure.

Likely an incorrect or invalid review comment.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 4, 2026

Greptile Summary

This PR updates the SettingsPage to handle async params (aligning with Next.js 15+ conventions) and changes the category detection logic from startsWith to includes.

Changes:

  • SettingsPage: Correctly makes the component async and awaits the Promise-based params before accessing the id
  • Navigation config: Changes category path matching from startsWith to includes

Issues Found:

  • The navigation config change breaks category detection for nested settings routes like /websites/123/settings/general, which now incorrectly match the /settings category instead of being recognized as website-specific settings

Confidence Score: 2/5

  • This PR introduces a critical navigation routing bug that will cause incorrect category selection for website settings pages
  • The SettingsPage changes are correct and safe, but the navigation-config change from startsWith to includes introduces a logic error. The new implementation incorrectly matches paths containing the pattern anywhere (e.g., /websites/123/settings/general matches /settings when it shouldn't). This will break the category sidebar for users navigating to website-specific settings, displaying the wrong category. The fix requires ensuring pattern matches occur at path boundaries, not just anywhere in the string.
  • apps/dashboard/components/layout/navigation/navigation-config.tsx - The getDefaultCategory function's pattern matching logic needs to be fixed to prevent false positives for nested paths.

Important Files Changed

Filename Overview
apps/dashboard/components/layout/navigation/navigation-config.tsx Navigation config category matching logic changed from startsWith to includes, which introduces a bug. Routes like /websites/123/settings/general now incorrectly trigger the /settings category matcher, breaking navigation for website-specific settings pages.
apps/dashboard/app/(main)/websites/[id]/settings/page.tsx Correctly updated to handle async params. The page is now an async component that awaits the Promise-based params before accessing them, which aligns with Next.js 15+ dynamic segment conventions.

Sequence Diagram

sequenceDiagram
    participant Client
    participant NextJS as Next.js Router
    participant SettingsPage
    participant NavConfig as Navigation Config

    Client->>NextJS: Navigate to /websites/[id]/settings
    NextJS->>SettingsPage: Render with params Promise
    SettingsPage->>SettingsPage: await params
    SettingsPage->>SettingsPage: Extract id
    SettingsPage->>NextJS: redirect(/websites/{id}/settings/general)
    
    NextJS->>NavConfig: getDefaultCategory(/websites/{id}/settings/general)
    Note over NavConfig: BUG: includes() matches /settings<br/>Should only match /settings routes
    NavConfig->>NavConfig: Incorrect category selection
    NavConfig-->>Client: Wrong sidebar category displayed
Loading

greptile-apps[bot]
greptile-apps bot reviewed Jan 4, 2026
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. apps/dashboard/components/layout/navigation/navigation-config.tsx, line 416 (link)

    logic: Changing from startsWith to includes introduces a bug. With includes, the path /websites/123/settings/general now incorrectly matches the /settings pattern, when it should only match when explicitly navigating to /settings/* routes. This breaks category detection for website settings pages. Consider adding a path boundary check:

    Or use a regex to ensure word boundary matching.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@izadoesdev izadoesdev merged commit c05f8e9 into databuddy-analytics:staging Jan 4, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sbansal1999 @izadoesdev